Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: restore installation of @node-rs/crc32 optional deps #984

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

achou11
Copy link
Member

@achou11 achou11 commented Jan 7, 2025

4f33bd0 updated the lockfile to remove the native binaries produced by @node-rs/crc32, which prevents me from locally running anything that touches yauzl-promise (which uses that module). This has happened in the past and I honestly don't really understand what the expected behavior should be or why this happens in general.

The modules in question are marked as optional deps in the published package, but not sure how npm decides to add/remove these from the lockfile. And if they're considered optional, not sure why @node-rs/crc32 complains if the relevant binaries are missing at runtime (i.e. not truly optional by definition).

Explicitly marking the module as a dependency for us seems to restore the lockfile so that the native binaries are included, but my guess is that this isn't the desired solution to the problem.

@achou11 achou11 requested a review from gmaclennan January 7, 2025 21:12
@awana-lockfile-bot
Copy link

package-lock.json changes

Click to toggle table visibility
Name Status Previous Current
@emnapi/core ADDED - 1.3.1
@emnapi/runtime ADDED - 1.3.1
@emnapi/wasi-threads ADDED - 1.0.1
@napi-rs/wasm-runtime ADDED - 0.2.6
@node-rs/crc32-android-arm-eabi ADDED - 1.10.6
@node-rs/crc32-android-arm64 ADDED - 1.10.6
@node-rs/crc32-darwin-arm64 ADDED - 1.10.6
@node-rs/crc32-darwin-x64 ADDED - 1.10.6
@node-rs/crc32-freebsd-x64 ADDED - 1.10.6
@node-rs/crc32-linux-arm-gnueabihf ADDED - 1.10.6
@node-rs/crc32-linux-arm64-gnu ADDED - 1.10.6
@node-rs/crc32-linux-arm64-musl ADDED - 1.10.6
@node-rs/crc32-linux-x64-gnu UPDATED 1.10.3 1.10.6
@node-rs/crc32-linux-x64-musl UPDATED 1.10.3 1.10.6
@node-rs/crc32-wasm32-wasi ADDED - 1.10.6
@node-rs/crc32-win32-arm64-msvc ADDED - 1.10.6
@node-rs/crc32-win32-ia32-msvc ADDED - 1.10.6
@node-rs/crc32-win32-x64-msvc ADDED - 1.10.6
@node-rs/crc32 UPDATED 1.10.3 1.10.6
@tybys/wasm-util ADDED - 0.9.0

@achou11
Copy link
Member Author

achou11 commented Jan 7, 2025

Ah, a hint!

https://github.com/napi-rs/node-rs/blob/11a9a1f37f9b42899071789f1232556420600983/packages/crc32/index.js#L358-L361

I've definitely tried the node_modules reinstallation approach, but that doesn't seem to work for me (given that the lockfile doesn't specify any of the binaries as deps)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant